Why not a multiaddr?

theres a TODO above explaining we can't get the Multiaddr type because of this issue microsoft/TypeScript#41672

Ah right! This is not nice as we will need to update this again after, I might wait for this to be solved before getting libp2p PR updating this merged in.
But we can move forward with this module here.

the only way to solve this now, is to do a bigger breaking change and default export the Multiaddr class directly. this would remove the ability to multiaddr('/ip..') and force new Multiaddr('')

should we do this ?

i can add a static method to the Multiaddr class called multiaddr and users would only need to change from const multiaddr = require('multiaddr) to const { multiaddr } = require('multiaddr)

I would like to have the constructor exported by default to be honest. But, I would wait on @jacobheun input before moving that way as this has a impact on a large number of modules.

yeah having the constructor exported directly makes a bunch of things easier and documentation looks a lot better.

I'm in favor of exporting the constructor, we should make sure we have clear migration docs for people. We should also add a static method for creation, this would make it easy to hot swap the two and allow us to be more picky about the constructor args. Since this is in the multiformats org it might be good to be consistent with the cid update effort, multiformats/js-multiformats#23, and call it asMultiaddr

I think it would make more sense to have Multiaddr.parse(string) static and make constructor @private. That way this turns into

And if we really want to smooth out transition we could have deprecated alias multiaddr.

My argument for discouraging new Multiadddr is because has tendency to grow into new Multiaddr(any) over time.

My personal opinion is to reserve constructors for property initializations, all the computation and logic should go into dedicated functions.

Edit: Added more details in the contstructor

Checking length is redundant

Can we use this breaking change as an opportunity to make it more type friendly and switch to more cleaner approach:

const symbol = Symbol.for('@multiformats/js-multiaddr/multiaddr')

class Multiaddr {
  /**
   * Parses string encoded multiaddr.
   * 
   * @param {string} addr
   * @returns {Multiaddr}
   */
  static parse(addr) {
    if (addr.length > 0 && addr.charAt(0) !== '/') {
      throw new Error(`multiaddr "${addr}" must start with a "/"`)
    }
    return new Multiaddr(codec.fromString(addr))
  }

  /**
   * Decodedes binary encoded multiaddr.
   * 
   * @param {Uint8Array} addr
   * @returns {Multiaddr}
   */
  static decode(addr) {
    return new Multiaddr(codec.fromBytes(addr))
  }

  static empty() {
    return Multiaddr.parse('')
  }

  /**
   * @typedef {Multiaddr|string|InstanceType<typeof String>|Uint8Array|null} ToMultiaddr
   * @param {ToMultiaddr} addr
   * @returns {Multiaddr}
   */
  static from(addr) {
    if (typeof addr === 'string') {
      return Multiaddr.parse(addr)
    } else if (addr instanceof Uint8Array) {
      return Multiaddr.decode(addr)
    } else if (Multiaddr.isMultiaddr(addr)) {
      return new Multiaddr(addr.bytes)
    } else if (addr === null) {
      return Multiaddr.empty()
    } else {
      throw new Error('addr must be a string, Buffer, or another Multiaddr')
    }
  }
  /**
   * @param {any} value
   * @returns {value is Multiaddr}
   */
  static isMultiaddr(value) {
    return Boolean(value && value[symbol])
  }
  /**
   * @private
   * @param {Uint8Array} bytes
   */
  constructor(bytes) {
    this.bytes = bytes
  }
}

Which has bunch of benifs:

1. When you read Multiadddr.parse(input) it is clear what the input without any tooling or side trips.
2. Constructor is dead simple
3. Each code path is also absolutely clear.
4. You still have .from to take any input when you don't know what you have.
5. No added overhead on each instantiation when you know your types.

For what it's worth that is where multiformats/cid ended up and feedback so far had been positive.

Can we turn that this into a getter please ?

class Multiaddr {
  get [symbol]() { 
    return true
  }
}

That is what we end up doing in other places.

Can we do this instead, I find it a lot easier to follow (and engines can optimize better)

toOptions() {
  const [,family,host, transport, port] =  this.toString().split('/')
  return {
    famility: family === 'ip4' ? 'ipv4' : 'ipv6',
    host,
    transport,
    port
  }
}

I think it would make sense to deprecate it to encourage transition.